[JAVA][#5172] Allow vendor json media types#5189
[JAVA][#5172] Allow vendor json media types#5189wing328 merged 9 commits intoswagger-api:masterfrom davidharrigan:FIX-5172
Conversation
davidharrigan
changed the title
|
Could you rebase your branch onto current master, and possibly regenerate the samples afterwards? I think some of the changes in the generated samples are already incorporated there, and this would make it easier to see what actually changed. |
| */ | ||
| public boolean isJsonMime(String mime) { | ||
| return mime != null && mime.matches("(?i)application\\/json(;.*)?"); | ||
| String jsonMime = "(?i)^(application/json|[^;/ \t]+/[^;/ \t]+[+]json)[ \t]*(;.*)?$"; |
There was a problem hiding this comment.
Are tab characters really allowed in media type names?
| assertTrue(apiClient.isJsonMime("example/foo+bar+json")); | ||
| assertTrue(apiClient.isJsonMime("example/foo+json;x;y")); | ||
| assertTrue(apiClient.isJsonMime("example/foo+json\t;")); | ||
| assertTrue(apiClient.isJsonMime("Example/fOO+JSON")); |
There was a problem hiding this comment.
If you want a real-live example, application/problem+json (from RFC 7807) is often used in APIs.
There was a problem hiding this comment.
Sure thing, I'll update these tests with some real-live example.
|
Looks like the failed test is fixed here: #5243 |
|
Yes, sorry for the update proposal, I didn't mean to cause more issues by that, but simply forgot that current master is "broken" (for sample generation) until #5243 is merged. |
|
no worries :) |
| public boolean isJsonMime(String mime) { | ||
| return mime != null && mime.matches("(?i)application\\/json(;.*)?"); | ||
| String jsonMime = "(?i)^(application/json|[^;/ \t]+/[^;/ \t]+[+]json)[ \t]*(;.*)?$"; | ||
| return mime != null && mime.matches(jsonMime); |
There was a problem hiding this comment.
Later I will file a PR to put these 2 lines into one without declaring a temporary variable.
| assertTrue(apiClient.isJsonMime("example/foo+bar+json")); | ||
| assertTrue(apiClient.isJsonMime("example/foo+json;x;y")); | ||
| assertTrue(apiClient.isJsonMime("example/foo+json\t;")); | ||
| assertTrue(apiClient.isJsonMime("Example/fOO+JSON")); |
There was a problem hiding this comment.
We'll leverage these test cases in other API clients' unit tests
|
@ePaul thanks for reviewing the change. @davidharrigan thanks for the contribution, which has been merged into master. |
) * [swagger-api#5172] Allow vendor json media types * Revert unnecessary diffs * Update petstore sample * Didn't run mvn after some edits * Rerun ' ./bin/java-petstore-all.sh' and './bin/security/java-petstore-okhttp-gson.sh' * Added more realistic test cases for isJsonMime
3 participants
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)2.3.0branch for breaking (non-backward compatible) changes.Description of the PR
Adds support for vendor media type JSON (ends with
+json) as specified in RFC-3839.PR made against master since the change is non-breaking and backwards compatible (still has the same support for
application/json).More details in the issue here: #5172